Skip to content

fix: Assert expected hooks for restricted method specifications#3993

Merged
FrederikBolding merged 5 commits into
mainfrom
fb/assert-hooks-for-restricted-methods
May 11, 2026
Merged

fix: Assert expected hooks for restricted method specifications#3993
FrederikBolding merged 5 commits into
mainfrom
fb/assert-hooks-for-restricted-methods

Conversation

@FrederikBolding
Copy link
Copy Markdown
Member

@FrederikBolding FrederikBolding commented May 8, 2026

Assert the expected hooks as part of buildSnapRestrictedMethodSpecifications to indicate missing or extraneous hooks being passed in to the developer.


Note

Medium Risk
Medium risk because it adds strict hook validation in buildSnapRestrictedMethodSpecifications, which can cause runtime errors for callers passing missing or extra hooks, and updates simulation wiring accordingly.

Overview
Adds hook contract enforcement to buildSnapRestrictedMethodSpecifications by computing the expected methodHooks from non-excluded restricted-method builders, calling assertExpectedHooks, and filtering excluded permissions before spec creation.

Updates tests to pass the full required hook set and adds coverage ensuring excluded permissions (e.g., snap_dialog) are omitted. Adjusts snaps simulation permission spec assembly to pass only the required hooks (including getSnapKeyring) and tweaks Jest coverage thresholds.

Reviewed by Cursor Bugbot for commit 6640a8d. Bugbot is set up for automated code reviews on this repo. Configure here.

@FrederikBolding FrederikBolding force-pushed the fb/assert-hooks-for-restricted-methods branch from cb920dd to 1453f3a Compare May 11, 2026 08:54
@FrederikBolding FrederikBolding marked this pull request as ready for review May 11, 2026 09:06
@FrederikBolding FrederikBolding requested a review from a team as a code owner May 11, 2026 09:06
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 7c9ad72. Configure here.

Comment thread packages/snaps-rpc-methods/src/permissions.ts
@codecov
Copy link
Copy Markdown

codecov Bot commented May 11, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.58%. Comparing base (253ace1) to head (6640a8d).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3993   +/-   ##
=======================================
  Coverage   98.58%   98.58%           
=======================================
  Files         427      427           
  Lines       12400    12408    +8     
  Branches     1948     1949    +1     
=======================================
+ Hits        12224    12232    +8     
  Misses        176      176           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@FrederikBolding FrederikBolding added this pull request to the merge queue May 11, 2026
Merged via the queue into main with commit 92ace80 May 11, 2026
129 checks passed
@FrederikBolding FrederikBolding deleted the fb/assert-hooks-for-restricted-methods branch May 11, 2026 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants